Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add cloud storage support for session start #3629

Merged
merged 28 commits into from
Jan 17, 2024

Conversation

Panaetius
Copy link
Member

@Panaetius Panaetius commented Sep 28, 2023

Also adds renku storage ls

adapted for SwissDataScienceCenter/renku-notebooks#1707 and related features

@coveralls
Copy link
Collaborator

coveralls commented Sep 28, 2023

Pull Request Test Coverage Report for Build 7553200191

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 85.167%

Totals Coverage Status
Change from base Build 7553017368: -0.3%
Covered Lines: 26636
Relevant Lines: 31275

💛 - Coveralls

@Panaetius Panaetius force-pushed the feat/rclone-storage-commands branch 2 times, most recently from b37e9e5 to debdfd6 Compare September 28, 2023 14:13
@Panaetius Panaetius force-pushed the feat/rclone-storage-commands branch from deb1302 to 23553b6 Compare November 9, 2023 15:13
@Panaetius Panaetius marked this pull request as ready for review November 10, 2023 08:46
@Panaetius Panaetius requested a review from a team as a code owner November 10, 2023 08:46
@Panaetius Panaetius requested a review from olevski November 13, 2023 13:32
@olevski
Copy link
Member

olevski commented Nov 17, 2023

@Panaetius I started reviewing this.

But I also wanted to try it out. So I made a project, added cloud storage and then cloned it locally. Then I ran renku storage ls and I got this error:

Please select an action by typing its name (open, print, ignore) [ignore]: 
Traceback (most recent call last):
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/bin/renku", line 6, in <module>
    sys.exit(cli())
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/tolevski/Documents/renku/renku-python/renku/ui/cli/exception_handler.py", line 139, in main
    self._handle_github()
  File "/Users/tolevski/Documents/renku/renku-python/renku/ui/cli/exception_handler.py", line 171, in _handle_github
    getattr(self, "_process_" + value)()
  File "/Users/tolevski/Documents/renku/renku-python/renku/ui/cli/exception_handler.py", line 131, in main
    return super().main(*args, **kwargs)
  File "/Users/tolevski/Documents/renku/renku-python/renku/ui/cli/exception_handler.py", line 90, in main
    return super().main(*args, **kwargs)
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/tolevski/Library/Caches/pypoetry/virtualenvs/renku-LaTIHLQX-py3.10/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/tolevski/Documents/renku/renku-python/renku/ui/cli/storage.py", line 59, in ls
    result = list_storage_command().build().execute()
  File "/Users/tolevski/Documents/renku/renku-python/renku/command/storage.py", line 22, in list_storage_command
    from renku.core.storage import list_storage
  File "/Users/tolevski/Documents/renku/renku-python/renku/core/storage.py", line 26, in <module>
    def list_storage(storage_service: IStorageService):
  File "pydantic/decorator.py", line 36, in pydantic.decorator.validate_arguments.validate
  File "pydantic/decorator.py", line 126, in pydantic.decorator.ValidatedFunction.__init__
  File "pydantic/decorator.py", line 264, in pydantic.decorator.ValidatedFunction.create_model
  File "pydantic/main.py", line 1026, in pydantic.main.create_model
  File "pydantic/main.py", line 197, in pydantic.main.ModelMetaclass.__new__
  File "pydantic/fields.py", line 506, in pydantic.fields.ModelField.infer
  File "pydantic/fields.py", line 436, in pydantic.fields.ModelField.__init__
  File "pydantic/fields.py", line 552, in pydantic.fields.ModelField.prepare
  File "pydantic/fields.py", line 639, in pydantic.fields.ModelField._type_analysis
  File "/Users/tolevski/.pyenv/versions/3.10.8/lib/python3.10/typing.py", line 1498, in __instancecheck__
    raise TypeError("Instance and class checks can only be used with"
TypeError: Instance and class checks can only be used with @runtime_checkable protocols

@olevski olevski reopened this Nov 17, 2023
@@ -261,6 +263,38 @@ def find_image(self, image_name: str, config: Optional[Dict[str, Any]]) -> bool:
== 200
)

def get_cloudstorage(self):
"""Get cloudstorage configured for the project."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not know where to put this but I think we need a warning in the docker provider that if there is any cloud storage in the project it will not be mounted. We dont have to implement this but we should let people now what the limitations are.

renku/ui/cli/storage.py Show resolved Hide resolved
Raises an exception for invalid storage.
"""
self._send_request(
"/storage_schema/validate", body=storage.configuration, method="POST", expected_response=[204]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the expected reponse for validate supposed to be really 204?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes when given a valid config it just returns an empty 204 and if invalid an appropriate 4xx code

renku/core/lfs.py Show resolved Hide resolved
renku/command/format/storage.py Show resolved Hide resolved
@Panaetius Panaetius force-pushed the feat/rclone-storage-commands branch from 325d93b to f3dd0d9 Compare December 6, 2023 09:03
@Panaetius Panaetius force-pushed the feat/rclone-storage-commands branch from 090c613 to f407d5a Compare December 6, 2023 12:35
Ralf Grubenmann added 2 commits December 7, 2023 12:04
@Panaetius Panaetius force-pushed the feat/rclone-storage-commands branch from 8f46103 to 7abb677 Compare December 7, 2023 14:29
@olevski olevski self-requested a review December 18, 2023 15:47
olevski
olevski previously approved these changes Dec 18, 2023
@Panaetius Panaetius enabled auto-merge (squash) December 20, 2023 08:33
olevski
olevski previously approved these changes Jan 17, 2024
@Panaetius Panaetius merged commit ec3173a into develop Jan 17, 2024
27 checks passed
@Panaetius Panaetius deleted the feat/rclone-storage-commands branch January 17, 2024 09:26
Panaetius pushed a commit that referenced this pull request Jan 17, 2024
* fix(service): accept commit-sha in config.show (#3685)

* feat: add cloud storage support for session start (#3629)

* chore: release v2.9.0

* update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants